fix: handle missing struct field in const evaluation instead of panicking#9812
fix: handle missing struct field in const evaluation instead of panicking#9812
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
This change is |
eytan-starkware
left a comment
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
eytan-starkware
left a comment
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
crates/cairo-lang-semantic/src/items/constant.rs line 728 at r1 (raw file):
.find(|(_, member_id)| m.id == *member_id) .map(|(expr_id, _)| self.evaluate(*expr_id)) // A missing field should have been caught by semantic validation.
Comment is a misleading. It was almost certainly caught, but allowed to continue processing
orizi
changed the base branch from
orizi/03-31-fix_unsupported_extern_fn_in_const
to
graphite-base/9812
orizi
force-pushed
the
orizi/03-31-fix_missing_struct_field_in_const
branch
from
0284f14 to
e37c2cf
Compare
orizi
force-pushed
the
graphite-base/9812
branch
from
6ef9148 to
5248475
Compare
orizi
left a comment
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-semantic/src/items/constant.rs line 728 at r1 (raw file):
Previously, eytan-starkware wrote…
Comment is a misleading. It was almost certainly caught, but allowed to continue processing
Done.
orizi
force-pushed
the
orizi/03-31-fix_missing_struct_field_in_const
branch
from
e37c2cf to
d4a3b71
Compare
3 participants
Summary
Fixed a panic in constant evaluation when struct literals have missing fields by returning a
ConstValue::Missinginstead of callingexpect(). Added a test case to verify proper error handling for statement-level const struct literals with missing fields.Type of change
Please check one:
Why is this change needed?
The constant evaluator was panicking when encountering struct literals with missing fields during constant evaluation. While semantic validation should catch these cases, the evaluator should handle them gracefully rather than crashing with an
expect()call.What was the behavior or documentation before?
The constant evaluator would panic with "Should have been caught by semantic validation" when processing struct literals with missing fields during constant evaluation.
What is the behavior or documentation after?
The constant evaluator now gracefully handles missing struct fields by returning
ConstValue::Missingand continues processing, allowing semantic validation to report the appropriate error messages to the user.Related issue or discussion (if any)
Fixes #9790
Additional context
The fix maintains the existing error reporting behavior while preventing crashes in the constant evaluator. The test case demonstrates that proper error messages are still generated for missing struct fields in const declarations.
Note
Low Risk
Small, targeted change that only affects error-path behavior in const evaluation and adds a diagnostic test; low chance of impacting successful const calculations.
Overview
Fixes a crash in constant evaluation for struct literals with missing fields by replacing an
expect()with a fallbackConstValue::Missing(skip_diagnostic()), avoiding cascading errors when semantic validation already reported the issue.Adds a regression test ensuring statement-level
conststruct literals with omitted members produce the expected "Missing member" diagnostic (and unused-const warning) instead of panicking.Written by Cursor Bugbot for commit d4a3b71. This will update automatically on new commits. Configure here.